Make the comment rule's test explicit: delete, don't shorten - #358

The comments rule has now been amended four times in four days -- ba31647
added it, #365 banned task references, #358 made the delete test explicit,
and the last commit here swapped in a silent-failure test. Each amendment
blacklisted whatever the previous violation looked like, so each one was
escapable by writing a differently-shaped bad comment, which is what kept
happening. Every PR in this stretch has needed a trim pass.

The silent-failure test was also wrong on its own terms. Two of the three
comments that have survived untouched in loggingproxy.py describe loud
failures -- a desync and a truncated capture -- and they are the best
comments in the repo.

What separates them from the trimmed ones is not the failure mode but the
information: they say something an intelligent reader could not have known
and cannot look up. That is the admission test now, and the prohibitions
collapse into a clause, since restating an option's name and re-telling
the branch's history both fail it for the same reason.

Shorter than what it replaces.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* ci: publish coverage to Codecov, one flag per tier

The job summary reports a number per run and nothing else; it has no
history, so the effect of the testing-framework work is invisible as a
trend. Codecov gets the same two tiers as separate flags, uploaded from
the coverage job's combined data, and graphs each over time.

relative_files is the load-bearing part. Without it the Cobertura XML
carries <source>/abs/path/to/vncdotool</source> with filename="api.py",
and nothing matching those against the repo tree finds a file --
coverage would have arrived and matched nothing.

Nothing gates: codecov.yml turns the project and patch statuses off, the
uploads set fail_ci_if_error false and are continue-on-error, so a
Codecov outage cannot redden a green run. That was the whole point of
the coverage work and a status check would undo it.

comment: false because the repo's own PR comment was already removed as
noise on the conversation; Codecov's is the same noise from a different
sender. carryforward per flag so a skipped fleet job leaves that flag at
its last value rather than reading as a collapse to zero.

Public repos upload without a token on codecov-action v5 and later.
Pinned to v7: v6.0.1 fixed a template injection the v5 line never got.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* ci: write the coverage XML outside the data file's namespace

combined/total.xml is itself a combined/total.* match, so `coverage html
--data-file=combined/total` picked the XML up as a parallel data file and
warned "file is not a database; Combined 0 files, 1 file errored" on the
first Codecov run.

Harmless there -- the report was already written -- but on a rerun a
stale XML sits in that namespace before the tier is reported, not after,
and then it is the report that errors. The XML goes in combined/xml/
instead, which no data file globs.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* ci: authenticate the Codecov upload with CODECOV_TOKEN

Tokenless uploads are documented to work for public repos, and they do
not: the first run was rejected with "Token required because branch is
protected" on a branch that is not protected, both tiers. It is a known
misreport (codecov-action#1580, #1709, #1795, #1800, #1918), so the token
is not optional and DEVELOP.rst no longer claims otherwise.

The secret is lifted to job-level env because `secrets` is not available
in a step-level `if` and `env` is. That guard skips the upload on a fork
PR, where the secret reads empty, rather than letting it fail: nothing
gates on coverage and the job summary carries the same numbers.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* docs: trim the Codecov comments to the repo's comment policy

The block over the upload steps ran seven lines and re-explained what
DEVELOP.rst already covers -- the tokenless rejection, the fork skip, why
nothing gates. It keeps the one clause a reader cannot get from the code,
that the token is not optional despite the docs, and points at the prose.

The rest lose their opening sentences, which restated the setting
underneath them rather than saying why it is set.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* docs: drop the codecov.yml header's restatement of off/off

"Coverage is reported, never gated" said what `project: off` and
`patch: off` say two lines below it. What the file cannot show is that
the setting is final rather than a placeholder, so that is all it says
now.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Delete every comment this branch added, and sharpen the rule that let them in

All seven were review-flagged, and the reason is the same for each: they
described something whose violation breaks loudly. Delete the token and
the upload fails with a message. Move the XML and coverage warns. Drop
relative_files and Codecov matches no files. None of that needs saying;
the failure says it, immediately, to whoever caused it.

CLAUDE.md's test -- a comment records a why the code cannot show -- did
not catch any of them, because any true statement passes it. Replaced
with the consequence test: a comment earns its place only when being
wrong would be silent, code that looks right and passes and is wrong
anyway. When the mistake is loud, the failure is the documentation.

Also drops the tokenless rejection from DEVELOP.rst. It was the story of
how the config was arrived at, which stopped mattering once it worked.

The section is shorter than the one it replaces. A rule about not
wasting the reader's time should not cost more to read.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* CLAUDE.md: admit a comment on surprise, not on how loudly it fails

The comments rule has now been amended four times in four days -- ba31647
added it, #365 banned task references, #358 made the delete test explicit,
and the last commit here swapped in a silent-failure test. Each amendment
blacklisted whatever the previous violation looked like, so each one was
escapable by writing a differently-shaped bad comment, which is what kept
happening. Every PR in this stretch has needed a trim pass.

The silent-failure test was also wrong on its own terms. Two of the three
comments that have survived untouched in loggingproxy.py describe loud
failures -- a desync and a truncated capture -- and they are the best
comments in the repo.

What separates them from the trimmed ones is not the failure mode but the
information: they say something an intelligent reader could not have known
and cannot look up. That is the admission test now, and the prohibitions
collapse into a clause, since restating an option's name and re-telling
the branch's history both fail it for the same reason.

Shorter than what it replaces.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>